-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
R4R: Staking Gaia-lite (ex LCD) refactor #1880
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a lot of duplicate code. Can we refactor here?
Which endpoint returns the the bond I have with any validator?
Idea: If it is still the case, let's resolve rational shares already in the LCD. It is harder to handle those on client side. (Maybe for another PR) |
@faboweb you can get a delegation with |
From the structure of the request I would expect it returns all bonding transactions I made to the validator, not the current bond. |
…edekunze/lcd_refactor pull develop
@ebuchman - want to sync on this one? |
@rigelrozanski addressed your comments. Are we good to merge now ? 😄 |
…smos-sdk into fedekunze/lcd_refactor Pull
x/stake/client/rest/query.go
Outdated
actions = append(actions, string(tags.ActionCompleteUnbonding)) | ||
actions = append(actions, string(tags.ActionBeginRedelegation)) | ||
actions = append(actions, string(tags.ActionCompleteRedelegation)) | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a switch statement here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since it doesn't has an expression I thought it was better with if
. But I can change it for better legibility
x/stake/client/rest/query.go
Outdated
// r.HandleFunc( | ||
// "/stake/delegators/{delegatorAddr}/validators/{validatorAddr}/txs", | ||
// stakingTxsHandlerFn(cliCtx, cdc), | ||
// ).Queries("type", "{type}").Methods("GET") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Delete all commented out code in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed
x/stake/client/rest/query.go
Outdated
// r.HandleFunc( | ||
// "/stake/delegators/{delegatorAddr}/validators", | ||
// delegatorValidatorHandlerFn(cliCtx, cdc), | ||
// ).Methods("GET") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delete all commented out code in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rigelrozanski This is a TODO
. Should we delete it or leave it as commented ? We already have the functionality but I spoke with @faboweb and agreed to leave it for later...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make an issue for that and delete the comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #1939
x/stake/client/rest/utils.go
Outdated
|
||
// Check if the delegator is bonded or redelegated to the validator | ||
keyDel := stake.GetDelegationKey(delegatorAddr, validatorAccAddr) | ||
// keyRed := stake.GetREDsByDelToValDstIndexKey(delegatorAddr, validatorAccAddr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh it was an attempt to create another key in order to get incoming redelegations, but Fabo already discovered how to do so without creating it. Will take it out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested ACK
These are breaking changes. For a complete list of refactored endpoints please check luniehq/lunie#1010
Left an issue (#1939) to finish the TODOs for a later Voyager sprint.
docs/
)PENDING.md
that include links to the relevant issue or PR that most accurately describes the change.cmd/gaia
andexamples/
For Admin Use: